-
Notifications
You must be signed in to change notification settings - Fork 157
git-add : Respect submodule ignore=all and only add changes with --force <path> #1987
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Welcome to GitGitGadgetHi @bicschneider, and welcome to GitGitGadget, the GitHub App to send patch series to the Git mailing list from GitHub Pull Requests. Please make sure that either:
You can CC potential reviewers by adding a footer to the PR description with the following syntax:
NOTE: DO NOT copy/paste your CC list from a previous GGG PR's description, Also, it is a good idea to review the commit messages one last time, as the Git project expects them in a quite specific form:
It is in general a good idea to await the automated test ("Checks") in this Pull Request before contributing the patches, e.g. to avoid trivial issues such as unportable code. Contributing the patchesBefore you can contribute the patches, your GitHub username needs to be added to the list of permitted users. Any already-permitted user can do that, by adding a comment to your PR of the form Both the person who commented An alternative is the channel
Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment If you want to see what email(s) would be sent for a After you submit, GitGitGadget will respond with another comment that contains the link to the cover letter mail in the Git mailing list archive. Please make sure to monitor the discussion in that thread and to address comments and suggestions (while the comments and suggestions will be mirrored into the PR by GitGitGadget, you will still want to reply via mail). If you do not want to subscribe to the Git mailing list just to be able to respond to a mail, you can download the mbox from the Git mailing list archive (click the curl -g --user "<EMailAddress>:<Password>" \
--url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txt To iterate on your change, i.e. send a revised patch or patch series, you will first want to (force-)push to the same branch. You probably also want to modify your Pull Request description (or title). It is a good idea to summarize the revision by adding something like this to the cover letter (read: by editing the first comment on the PR, i.e. the PR description):
To send a new iteration, just add another PR comment with the contents: Need help?New contributors who want advice are encouraged to join git-mentoring@googlegroups.com, where volunteers who regularly contribute to Git are willing to answer newbie questions, give advice, or otherwise provide mentoring to interested contributors. You must join in order to post or view messages, but anyone can join. You may also be able to find help in real time in the developer IRC channel, |
There are issues in commit a3105e7: |
There are issues in commit b2d5354: |
There are issues in commit 8348a4a: |
There are issues in commit 6657438: |
There are issues in commit c2d703e: |
There are issues in commit a56d8b8: |
There are issues in commit 79598c6: |
There are issues in commit 1b8d93b: |
There are issues in commit e45daed: |
/allow |
User bicschneider is now allowed to use GitGitGadget. WARNING: bicschneider has no public email address set on GitHub; GitGitGadget needs an email address to Cc: you on your contribution, so that you receive any feedback on the Git mailing list. Go to https://github.com/settings/profile to make your preferred email public to let GitGitGadget know which email address to use. |
@bicschneider the Git project is very particular about the form of the commit messages it accepts, and it is outright hostile towards contributors who send patches that introduce white-space problems even if those are fixed by later patches in the same patch series (they demand that the fixes are squashed into the appropriate patches, which I recommend you do before Further, they are adamant about the Finally, you can increase the chances for a pleasant review on the Git mailing list dramatically by imitating the rather verbose style of Git's own commit messages. There is some really useful guidance here, in particular the part where it recommends to ensure that the following are covered by the commit message:
|
e45daed
to
d94d2d5
Compare
There are issues in commit fa7cf4e: |
There are issues in commit 78c6a1a: |
There are issues in commit d94d2d5: |
ee01742
to
cd3e2a3
Compare
@bicschneider you needn't worry about the |
d636a89
to
d61866b
Compare
The ignored_too parameter is added to the function add_files_to_cache for usage of explicit updating the index for the updated submodule using the explicit patchspec to the submodule. Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
Given the submdule configuration is ignore=all then only update the submdule if the --force option is given and the submodule is explicit given in the pathspec. A message is printed (like ignored files) guiding the user to use the --force flag if the user has explicitely want to update the submodule reference. The reason for the change is support submodule branch tracking or similar and git status state nothing and git add should not add either. The workflow is more logic and similar to regular ignored files even the submodule is already tracked. The change opens up a lot of possibilities for submodules to be used more freely and a like the repo tool. A submodule can be added for many more reason and loosely coupled dependencies to the super repo which often gives the friction of handle the explicit commits and updates without the need for tracking the submodule sha1 by sha1. Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
…ario The tests verify that the submodule behavior is intact and updating the config with ignore=all also behaves as intended with configuration in .gitmodules and configuration given on the command line. Testfile is added to meson.build for execution. Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
There are tests that rely on "git add <submodule>" also adds it. A --force is needed with this enhancement hence they are added accordingly in these tests. Updated tests: - t1013-read-tree-submodule.sh ( fixed in: t/lib-submodule-update.sh ) - t7406-submodule-update.sh - t7508-status.sh Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
- The add --force doc: append additionel info about the submodule functionality. - The submodule config ignore=all now need --force in order to update the index. Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com>
d61866b
to
80907de
Compare
The feature of configuring a submodule to "ignore=all" is nicely respected in commands "status" and "diff". However the "add" command does not respect the configuration the same way. The behavior is problematic for the logic between status/diff and add. Secondly it makes it problematic to track branches in the submodule configuration as developers unintentionally keeps add submodule updates and get conflicts for no intentional reason. Both adds unnecessary friction to the usage of submodules.
The patches implement the same logical behavior for ignore=all submodules as regular ignored files. The status now does not show any diff - nor will the add command update the reference submodule reference. If you add the submodule path which is ignore=all then you are presented with a message that you need to use the --force option. The branch=, ignore=all (and update=none) now works great with update --remote, but developers does not have to consider changes in the updates of the submodule sha1. The implementation removes a friction of working with submodules and can be used like the repo tool with branches configured. The submodule status report could be used for build/release documentation for reproduction of a setup.
A few tests used the adding of submodules without --force, hence they have been updated to use the --force option.